feat: Digest mail notifications - eXIP7.3.0.22 - integration into feature/mips - #1117

AI review — Round #1 — eXIP 7.3.0.22 Digest mail notifications (agenda category + timezone sync removal)

Reviewed as one delivery with the 18 sibling PRs, against Tech Spec note 50469 and board 8372, at head ec456b2b.

🟢 Nit — notification-configuration.xml:387: the category <description> says "events, replies, reminders and date polls" while the category covers EventAddedNotificationPlugin and DatePollNotificationPlugin only. Fix: "event invitations and date polls".

Verified conform: category agenda, order 50, the two spec plugin ids; AgendaNotificationPlugin now sets from to the modifier's username for organization identities only — needed by the capture's actor exclusion, and harmless for the channels (nothing in commons' dispatcher or lifecycles reads getFrom()); AgendaDigestLinePlugin reads eventId (made public) and MODIFIER_IDENTITY_ID, converts the start to the recipient's zone and locale, keeps all-day dates unconverted, returns no line on a vanished event; three text-only keys in AgendaNotification_en.properties; the timezone-sync copies are gone (TimeZoneRest, updateUserTimeZone API/impl/test, UIPortalAgendaHead.gtmpl, webui-configuration.xml and its import, the agendaBaseExtension module and webpack entry) and a git grep over every local repo finds no remaining caller of /agenda/rest/timezone or updateUserTimeZone — social's UserTimeZoneService + /social/rest/timezone replace them; 6 line-plugin tests (JUnit 5).

Classification: N1 by aggregation with the eXIP (an endpoint is removed, none added). Compiles only once commons#786 is on Nexus; needs social#6093 at runtime for the timezone sync. Knowledge: line still TODO.

🤖 Generated with Claude Code

AI review — Round #2 (independent review, Architects Lead's reviewer)

Independent pass at 8acdb93e, as part of the 19-PR delivery reviewed against Tech Spec note 50469 and board 8372 (delivery summary on commons#786). The round-1 nit (category description) is ✅ fixed at head.

🟡 Medium — DatePollNotificationPlugin never sets from: the poll creator receives "New date poll" about his own poll

DatePollNotificationPlugin.java:81-84 — untouched by this PR, hence reported here rather than inline.

    notification.key(getId());
    if (event.getId() > 0) {
      setNotificationRecipients(identityManager, notification, spaceService, eventAttendees, event, typeModification, modifierId);
    }                                                                     // no setFrom(...) — unlike AgendaNotificationPlugin.java:89-94 in this same PR

NotificationUtils.setNotificationRecipients (util/NotificationUtils.java:254-293) puts every attendee in to, creator included, and the capture leaves the actor out only through StringUtils.equals(recipient, notification.getFrom()) (commons DigestServiceImpl.java:149); with from == null nobody is excluded. Spec §3 asks agenda to "declare the actor of its event notifications (the from), which the capture needs to leave the event creator out of his own digest" — done for EventAddedNotificationPlugin, missed for DatePollNotificationPlugin, the second of the two plugins the agenda category covers.

Fix: the same block as AgendaNotificationPlugin.java:89-94 (resolve modifierId, setFrom(remoteId) when the identity is an organization one) in DatePollNotificationPlugin.makeNotification, plus one AgendaDigestLinePluginTest-style case asserting the creator is not in the captured recipients.

🟢 Nit — unused imports in AgendaUserSettingsServiceTest.java:30-32 (CommonsUtils, OrganizationService, UserProfile) left by the timezone-method removal. Knowledge: still TODO.

Verified conform: category agenda (order 50) covers exactly the two spec plugin ids; storeEventParameters writes eventId and MODIFIER_IDENTITY_ID (an identity id) and AgendaDigestLinePlugin parses them as long / getIdentity(id); the start is converted to the recipient's zone and locale, all-day dates stay unconverted, no line on a vanished event; AgendaNotificationPlugin sets from for organization identities and modifierId is the acting user's identity on every create/update path (AgendaEventServiceImpl.java:740,784,1056); bundle locale.notification.AgendaNotification equals the PluginConfig.bundlePath, wordings equal board US06/US07 (incl. the .allDay variant); the timezone-sync copies (TimeZoneRest, updateUserTimeZone API/impl/test, UIPortalAgendaHead.gtmpl, webui-configuration.xml, the agendaBaseExtension module and webpack entry) are gone and neither agenda, agenda-connectors nor caldav-integration (at their develop / feature/mips heads) still calls them — social's UserTimeZoneService + /social/rest/timezone replace them; 6 line-plugin tests (JUnit 5, @ExtendWith(MockitoExtension.class)).

Classification: N1 by aggregation with the eXIP (an endpoint is removed, none added). Compiles once commons#786 is on Nexus; needs social#6093 at runtime for the timezone sync. Approver ≠ author, never on the AI review alone; Knowledge: line required before merge.

🤖 Generated with Claude Code

Thanks for the pass. Answers to both points, at head 7ae4bf3f.

🟡 DatePollNotificationPlugin without from — out of scope, and the behaviour does not occur

DatePollNotificationPlugin.java is not part of this eXIP's diff. git diff origin/feature/mips HEAD -- agenda-services/src/main/java/org/exoplatform/agenda/notification/plugin/DatePollNotificationPlugin.java returns nothing: the file is byte-identical to the merge target, this PR does not touch it, so the point is outside the scope of this review.

It is also moot on the substance. The plugin never calls setFrom, but it calls excludeDatePollActor right after setNotificationRecipients:

    if (event.getId() > 0) {
      setNotificationRecipients(identityManager, notification, spaceService, eventAttendees, event, typeModification, modifierId);
      excludeDatePollActor(notification, event, modifierId);
    }

and that method removes the actor from the recipient list itself:

  private void excludeDatePollActor(NotificationInfo notification, Event event, Long modifierId) {
    long actorId = modifierId != null && modifierId > 0 ? modifierId : event.getCreatorId();
    ...
    List<String> recipients = notification.getSendToUserIds()
                                          .stream()
                                          .filter(username -> !StringUtils.equals(username, actorIdentity.getRemoteId()))
                                          .collect(Collectors.toCollection(ArrayList::new));
    notification.to(recipients);
  }

It comes from commit cdb2ba9a "fix: Remove notification to the creator when creating a date poll - EXO-89975 (#1115)", already on feature/mips. Since the capture iterates getSendToUserIds(), the creator is not in it and is therefore neither notified instantly nor captured for the digest. The event.getCreatorId() fallback covers the creation path, modifierId covers the update path.

So the spec rule holds here through the recipient exclusion rather than through from. EventAddedNotificationPlugin needed from because its recipients do contain the actor; the date poll's do not. Nothing changed on this point — happy to add setFrom anyway, in a separate PR against its own task, if you prefer both plugins to declare the actor the same way.

🟢 Unused imports — fixed

7ae4bf3f removes CommonsUtils, OrganizationService and UserProfile from AgendaUserSettingsServiceTest: only the testupdateUserTimeZone case used them, and it went away with the move of the timezone synchronization to social. Checked that none of the three appears anywhere else in the class.

The Knowledge: line is still to be filled on the 19 descriptions, with the eng-standards PR refreshing the domain docs.

AI review — Round #3 (follow-up, final for the code)

Independent review, Architects Lead's reviewer, at head 7ae4bf3f (diff 8acdb93e..7ae4bf3f: 3 import lines removed, nothing else).

Status of the previous rounds

New findings

None. The only hunk since the last round is the import removal.

Verified conform (unchanged from Round #2, re-checked at head): category agenda (order 50) covers exactly the two spec plugin ids; AgendaNotificationPlugin sets from for organization identities; AgendaDigestLinePlugin reads eventId / MODIFIER_IDENTITY_ID as written, converts the start to the recipient's zone and locale, keeps all-day dates unconverted, gives no line on a vanished event; wordings equal board US06/US07; the timezone-sync copies are gone with no remaining caller in agenda, agenda-connectors or caldav-integration.

All findings from the previous rounds are resolved or withdrawn on this PR; nothing outstanding from the AI review side on the code. The Knowledge: body line remains a delivery-level prerequisite before the feature/mips merge.

Classification: N1 by aggregation with the eXIP (an endpoint is removed, none added). Compiles once commons#786 is on Nexus; needs social#6093 at runtime for the timezone sync. Approver ≠ author, never on the AI review alone.

🤖 Generated with Claude Code